-
Notifications
You must be signed in to change notification settings - Fork 103
Lesson 62 (wait notify) #107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: for-pr
Are you sure you want to change the base?
Lesson 62 (wait notify) #107
Conversation
| private String message; | ||
| private boolean isReceived = false; | ||
|
|
||
| public Message() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
зачем явно прописывать конструктор по умолчанию?
| wait(); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread() | ||
| .interrupt(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
зачем?
| isReceived = true; | ||
|
|
||
| notifyAll(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Размещать функциональность отправки и получения сообщения в самом Message - грубая ошибка. Завтра я захочу логику отправки-получения переделать через брокер сообщений или еще как-то - почему я при этом должен менять саму сущность "сообщение"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
То есть текущий класс надо либо переименовать, либо выделить класс логики, который будет отвечать за обработку сообщения
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Но я бы просто вынес отправку и получение в логику отдельных сущностей - тогда любой потенциальный полиморфизм будет реализовать проще
|
|
||
| public class MessageReceiver implements Runnable { | ||
| private final Message message; | ||
| private final Consumer<String> stringConsumer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Интересная идея. Но не всегда корректная на практике. В данном случае класс, создающий сущность получателя (или отправителя), должен знать, что будет происходить при отправке или получении. Чаще всего это не будет его ответственностью
| message.send(stringSupplier.get()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вердикт: технические решения местами интересные, с зонами ответственности классов перемудрил
| @Override | ||
| public void run() { | ||
| while (!Thread.currentThread() | ||
| .isInterrupted()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лишний перенос строки
| ; | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread() | ||
| .interrupt(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
|
|
||
| logger.log("Остаток товаров на складе - %d".formatted(depot.getStock())); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Хорошая идея. Но Depot тут определенно лишний, из-за него слушатель получает сильно много ответственности. Как вариант - можно пост-эффект тоже передавать лямбдой, этой будет логичнее
Ну и с названием класса можно подумать, ибо сейчас оно не отражает суть. Но это не так страшно
| if ("finish".equalsIgnoreCase(stringSupplier.get())) { | ||
| break; | ||
| } | ||
| ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
| Thread.sleep(Duration.ofSeconds(1)); | ||
|
|
||
| if ("finish".equalsIgnoreCase(stringSupplier.get())) { | ||
| break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
прохладная история, если есть флаг остановки
| int surplus = 0; | ||
|
|
||
| while (!Thread.currentThread() | ||
| .isInterrupted()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лишний перенос
| surplus = intSupplier.getAsInt(); | ||
| } | ||
|
|
||
| Thread.sleep(Duration.ofSeconds(random.nextInt(1, 4))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не помню в условии требований к простою у покупателей или поставщиков
| } | ||
|
|
||
| logger.log("Остаток товаров - %d. %s пытается купить %d товаров.".formatted(stock, Thread.currentThread() | ||
| .getName(), count)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лишний перенос
| } | ||
| } | ||
|
|
||
| logger.log("Остаток товаров - %d. %s пытается купить %d товаров.".formatted(stock, Thread.currentThread() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не логичнее в методы логгера докрутить varargs и дать возможность передавать параметры для форматирования? Чтобы не форматировать вручную в бизнес-коде
No description provided.